Update LDK dependency for splicing, http, and wallet_utils#794
Update LDK dependency for splicing, http, and wallet_utils#794jkczyz wants to merge 4 commits intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
| Ok(txid) | ||
| } | ||
|
|
||
| pub(crate) fn select_confirmed_utxos( |
There was a problem hiding this comment.
@tnull I'm wondering if we should implement CoinSelectionSource and use this rather than using LdkWallet for UTXO selection. It would prevent double persistence when we need a change output and the need for an explicit persist method after selection.
There was a problem hiding this comment.
Yeah, that seems preferable, if we have everything we need in the API by now?
There was a problem hiding this comment.
Right, I think the question may be whether we use our own CoinSelectionSource for anchor bumping, too. It depends on how UTXO locking will work. LdkWallet manages whether or not to try to double-spend based on the claim id, whereas for splicing we may accidentally double spend one of those if using our own CoinSelectionSource. Pushed a fixup commit implementing CoinSelectionSource for splicing.
There was a problem hiding this comment.
Huh, well the branch shows the fixup. Not sure why the PR hasn't updated.
There was a problem hiding this comment.
Right, I think the question may be whether we use our own CoinSelectionSource for anchor bumping, too.
It would be great to reuse the same codepaths, IMO. That is, if the API now permits for it.
It depends on how UTXO locking will work. LdkWallet manages whether or not to try to double-spend based on the claim id, whereas for splicing we may accidentally double spend one of those if using our own CoinSelectionSource.
Right, we're still waiting on bitcoindevkit/bdk_wallet#259 to ship with BDK 3.0. For now I'm not sure if we'd then need to also do some tracking based on ClaimId.
There was a problem hiding this comment.
Discussed offline. We'll use the current approach and then once BDK ships a new release we can revisit coin selection for anchor bumping.
Alternatives would be to either (1) drop the fixup such that we use LdkWallet for both coin selections or (2) maintain a locked UTXO list for splice coin selection and filter WalletSource::list_confirmed_utxos so that anchor bumping doesn't double spend splicing.
| Ok(txid) | ||
| } | ||
|
|
||
| pub(crate) fn select_confirmed_utxos( |
There was a problem hiding this comment.
Yeah, that seems preferable, if we have everything we need in the API by now?
4c01c07 to
4891c0b
Compare
7ccaa2a to
01102e6
Compare
There was a problem hiding this comment.
Seems this should be based on lightningdevkit/rust-lightning#4382 as it now has landed, as is the next related breaking change?
tnull
left a comment
There was a problem hiding this comment.
Changes LGTM, but as mentioned above it would be good to add a second commit to bump once more and include latest wallet-related API fixes, so that after this PR we can build against current LDK main again.
| fn sign_psbt<'a>( | ||
| &'a self, psbt: Psbt, | ||
| ) -> impl Future<Output = Result<Transaction, ()>> + Send + 'a { | ||
| debug_assert!(false); |
There was a problem hiding this comment.
It's only used for anchor bumping. For splicing, we provide signatures when handling FundingTransactionReadyForSigning and calling ChannelManager::funding_transaction_signed.
Yeah, unfortunately lightningdevkit/rust-lightning#4350 landed between the two PRs. Working on updating now. |
Update splice_in/splice_out to use new LDK two-phase funding API The LDK dependency bump introduced a new splicing API that separates negotiation from coin selection, letting LDK handle transaction construction internally rather than requiring manual UTXO selection and change address management. Generated with the assistance of AI (Claude Code). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The latest LDK dependency bump replaced the custom HTTP implementation in lightning-block-sync with the `bitreq` crate, changing constructor signatures, error types, and response parsing constraints. Update the bitcoind chain source to use the new string-based URL constructors and structured error enums. Generated with the assistance of AI (Claude Code). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
01102e6 to
f78aa6c
Compare
splice_in/splice_out to use new LDK two-phase funding API |
Ok, I changed the branch I used for updating |
|
May need to rebase to fix CI. Looking... |
Ah, actually looks like we still need to update for lightningdevkit/rust-lightning#4425. Will update shortly. |
The macro was removed in favor of a function.
Some types previously existing in lightning::events::bump_transaction have been moved to lightning::util::wallet_utils.
f78aa6c to
ad6c554
Compare
The LDK dependency bump introduced a new splicing API that separates negotiation from coin selection (lightningdevkit/rust-lightning#4290), letting LDK handle transaction construction internally rather than requiring manual UTXO selection and change address management.
Also handles changes to
lightning-block-syncmoving tobitreq(lightningdevkit/rust-lightning#4350), making a test macro a function (lightningdevkit/rust-lightning#4425), and moving some coin selection utils towallet_utils(lightningdevkit/rust-lightning#4382).